Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Scala 3 perf improvements, consolidate rep implementations #273

Merged
merged 14 commits into from
Mar 3, 2023
Merged

Scala 3 perf improvements, consolidate rep implementations #273

merged 14 commits into from
Mar 3, 2023

Conversation

lihaoyi
Copy link
Member

@lihaoyi lihaoyi commented Mar 3, 2023 •
edited
Loading

  1. Previously .rep() never inlined, and .repX() would always inline and fail with a compile error if it was not possible. This should make both perform inlining at a best-effort level, giving us the best of both worlds

  2. I saw a lot of calls to List() turning up in my JProfile, replaced them with raw :: calls. Seems like this is optmized automatically in Scala 2, but not in Scala 3 List(...) builder missing Scala 2 optimization to avoid intermediate array scala/scala3#17035

  3. Lift whitespace NoWhitespace check in .rep to compile time, like we do in ~.

  4. Combine all the menagerie of .rep implementations into one Scala 3 macro. Scala 2 is still a bit of a mess, cleaning that up is left for future

  5. Inlined the character checking in literalStrMacro to avoid allocating an intermediate function to call

Seems to provide a small but measurable performance boost for successful parses, and a significant performance boost for failures (where we allocate a lot more lists as part of error reporting). Not quite enough to catch up to Scala 2 performance, but brings us maybe half way there.

Scala 3 before:

JsonnetParse Benchmark
Max time - 10000 ms. Iterations - 5.
Iteration 1
Benchmark 0. Result: 9331
Benchmark 1. Result: 1546
Iteration 2
Benchmark 0. Result: 7715
Benchmark 1. Result: 1947
Iteration 3
Benchmark 0. Result: 7549
Benchmark 1. Result: 1976
Iteration 4
Benchmark 0. Result: 7613
Benchmark 1. Result: 1953
Iteration 5
Benchmark 0. Result: 7686
Benchmark 1. Result: 1907

Scala 3 after:

JsonnetParse Benchmark
Max time - 10000 ms. Iterations - 5.
Iteration 1
Benchmark 0. Result: 9466
Benchmark 1. Result: 2611
Iteration 2
Benchmark 0. Result: 8152
Benchmark 1. Result: 2832
Iteration 3
Benchmark 0. Result: 8139
Benchmark 1. Result: 2799
Iteration 4
Benchmark 0. Result: 8020
Benchmark 1. Result: 2844
Iteration 5
Benchmark 0. Result: 8126
Benchmark 1. Result: 2868

Scala 2.13:

JsonnetParse Benchmark
Max time - 10000 ms. Iterations - 5.
Iteration 1
Benchmark 0. Result: 9850
Benchmark 1. Result: 2773
Iteration 2
Benchmark 0. Result: 8781
Benchmark 1. Result: 2912
Iteration 3
Benchmark 0. Result: 8742
Benchmark 1. Result: 2916
Iteration 4
Benchmark 0. Result: 8782
Benchmark 1. Result: 2912
Iteration 5
Benchmark 0. Result: 8703
Benchmark 1. Result: 2940

@lihaoyi lihaoyi changed the title Tweak inlining to try and be robust against static and dynamic values Tweak inlining to try and be robust against static and dynamic values, & some perf improvements Mar 3, 2023
@lihaoyi lihaoyi requested review from lolgab and lefou March 3, 2023 02:57
@lihaoyi lihaoyi changed the title Tweak inlining to try and be robust against static and dynamic values, & some perf improvements Scala 3 perf improvements Mar 3, 2023
@lihaoyi lihaoyi mentioned this pull request Mar 3, 2023
Closed
@lihaoyi lihaoyi changed the title Scala 3 perf improvements Scala 3 perf improvements, consolidate rep implementations Mar 3, 2023
lefou
lefou reviewed Mar 3, 2023
View reviewed changes
@lihaoyi lihaoyi merged commit 0a3de65 into master Mar 3, 2023
@lefou lefou deleted the inlining branch March 3, 2023 11:35
@lefou lefou added this to the 3.0.0 milestone Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lefou lefou lefou left review comments

@lolgab lolgab lolgab approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@lihaoyi @lefou @lolgab